Re-arm the flush timer after a broker reconnect - #108
Merged
clokep merged 4 commits intoAug 26, 2026
Conversation
Strategy() runs every time the consumer (re)starts, including after a lost broker connection is re-established. The flush timer (self._tref) is bound to the consumer's event-loop hub and the buffered requests reference the consumer's channel, both of which are replaced on reconnect. The timer reference was kept across reconnects, so it was never re-armed on the new hub; when the broker prefetch is smaller than flush_every the worker silently stops flushing batches and wedges, holding its prefetched messages unacked until it is restarted. Reset the per-consumer state (timer, buffers, count) at the start of Strategy() so the next message re-arms the flush timer on the new hub. Buffered requests belong to the old connection and are redelivered by the broker, so they are dropped. Fixes clokep#99 Co-authored-by: Cursor <cursoragent@cursor.com>
clokep
reviewed
Aug 26, 2026
clokep
reviewed
Aug 26, 2026
Co-authored-by: Patrick Cloke <clokep@users.noreply.github.com>
clokep
approved these changes
Aug 26, 2026
Two failures under the repo's mypy config (warn_return_any, warn_unreachable): - Celery's task decorator is untyped, so the decorated function is Any and returning it from a helper annotated `-> Batches` triggers no-any-return. Make the conversion explicit with a cast. - `_tref` is declared `Optional[Timer]`, but Timer is untyped so the declared type is `Union[Any, None]`. Assigning a MagicMock narrowed the attribute to MagicMock, making `assert task._tref is None` provably false and every statement after it unreachable. Typing the mock as Any keeps the attribute at its declared Optional type.
Owner
|
Looks like GitHub Actions is having a day again. 😞 |
Owner
|
Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the long-standing "RMQ reconnection" wedge (#99): after the broker connection is lost and re-established, a
Batchesworker can stop flushing batches entirely and sit holding its prefetched messages unacked until it is restarted. I dug into the root cause and have a one-spot fix inBatches.Strategy(). Full analysis (with a minimal repro) is in #107.Root cause
Strategy()runs every time the consumer (re)starts, including after a reconnect. The flush timer is armed lazily inside the message handler:self._treflives on the task instance, which survives reconnects, but the timer it points at is bound to the consumer's old event-loop hub. After a reconnect Celery rebuilds the consumer (new hub, new channel) and callsStrategy()again, butself._trefis still set, so theif self._tref is Noneguard never re-arms the timer on the new hub.The interval-based flush is then dead. The only remaining flush path is the count-based one (
flush_every). With the common broker setupworker_prefetch_multiplier=1(so the broker delivers fewer thanflush_everymessages and waits for acks), that count threshold is never reached either, so the worker buffers its prefetched messages forever without acking and looks "connected but idle". This matches the symptoms reported in #99.Fix
Reset the per-consumer state (timer, buffers, count) at the start of
Strategy(), before the new message handler closes overself._buffer. The next message then re-arms the flush timer on the new hub via the existing guard. Buffered requests belong to the old connection and are redelivered by the broker, so they are dropped.Because
Strategy()only runs at consumer (re)start — not per message — there is no steady-state overhead, and on a normal first start the reset is a no-op (the buffers are already empty and_trefisNone).Testing
t/unit/test_strategy.pycovering both the reconnect reset (stale timer cancelled,_trefcleared so it re-arms, buffers/count reset) and the first-start no-op path.pytest,mypypass locally.Batchesworker withworker_prefetch_multiplier=1wedges onmainafter a broker restart and recovers on its own with this change, including under sustained memory/disk alarms.Closes #99